Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: move patch function into utils to improve bundling #1631

Merged
merged 5 commits into from
Feb 6, 2025

Conversation

pauldambra
Copy link
Contributor

split out of #1594 which I managed to break :/

the current way that patch is bundled means the console recorder plugin includes all of rrweb in order to include patch

image

in the original PR we saw:

move the patch function into the existing @rrweb/utils package fixes this

(I've only proven the bundle is fixed - i'm hoping CI proves the recorder still works 😅 )

just looking at bundle size this reduces the console recorder plugin from 255kb to 15kb

Copy link

changeset-bot bot commented Jan 17, 2025

🦋 Changeset detected

Latest commit: b1ca310

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 19 packages
Name Type
@rrweb/rrweb-plugin-console-record Patch
@rrweb/record Patch
rrweb Patch
@rrweb/utils Patch
rrweb-snapshot Patch
rrdom Patch
rrdom-nodejs Patch
rrweb-player Patch
@rrweb/all Patch
@rrweb/replay Patch
@rrweb/types Patch
@rrweb/packer Patch
@rrweb/web-extension Patch
rrvideo Patch
@rrweb/rrweb-plugin-console-replay Patch
@rrweb/rrweb-plugin-sequential-id-record Patch
@rrweb/rrweb-plugin-sequential-id-replay Patch
@rrweb/rrweb-plugin-canvas-webrtc-record Patch
@rrweb/rrweb-plugin-canvas-webrtc-replay Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pauldambra
Copy link
Contributor Author

cc @Juice10 since you reviewed the original PR I managed to break :)

@pauldambra
Copy link
Contributor Author

in PostHog CI with bundle tracking active this PR gives us

Screenshot 2025-01-20 at 16 59 46

eoghanmurray
eoghanmurray previously approved these changes Feb 4, 2025
Copy link
Contributor

@eoghanmurray eoghanmurray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm approving to see if it gets the tests to run

@eoghanmurray eoghanmurray dismissed their stale review February 4, 2025 14:09

didn't work

@Juice10
Copy link
Contributor

Juice10 commented Feb 6, 2025

Error: compressed-size-action was unable to comment on your PR.
This can happen for PR's originating from a fork without write permissions.
You can copy the size table directly into a comment using the markdown below:

Size Change: -1.58 MB (-13.33%) 👏

Total Size: 10.3 MB

Filename Size Change
packages/all/dist/all.cjs 554 kB +588 B (+0.11%)
packages/all/dist/all.js 554 kB +588 B (+0.11%)
packages/all/dist/all.umd.cjs 557 kB +593 B (+0.11%)
packages/all/dist/all.umd.min.cjs 271 kB +252 B (+0.09%)
packages/plugins/rrweb-plugin-console-record/dist/rrweb-plugin-console-record.cjs 15 kB -240 kB (-94.11%) 🏆
packages/plugins/rrweb-plugin-console-record/dist/rrweb-plugin-console-record.js 14.9 kB -240 kB (-94.16%) 🏆
packages/plugins/rrweb-plugin-console-record/dist/rrweb-plugin-console-record.umd.cjs 16.1 kB -241 kB (-93.76%) 🏆
packages/plugins/rrweb-plugin-console-record/dist/rrweb-plugin-console-record.umd.min.cjs 8.03 kB -119 kB (-93.67%) 🏆
packages/plugins/rrweb-plugin-console-replay/dist/rrweb-plugin-console-replay.cjs 5.01 kB -216 kB (-97.73%) 🏆
packages/plugins/rrweb-plugin-console-replay/dist/rrweb-plugin-console-replay.js 4.9 kB -216 kB (-97.78%) 🏆
packages/plugins/rrweb-plugin-console-replay/dist/rrweb-plugin-console-replay.umd.cjs 6.07 kB -218 kB (-97.29%) 🏆
packages/plugins/rrweb-plugin-console-replay/dist/rrweb-plugin-console-replay.umd.min.cjs 3.27 kB -107 kB (-97.04%) 🏆
packages/record/dist/record.cjs 376 kB +597 B (+0.16%)
packages/record/dist/record.js 376 kB +597 B (+0.16%)
packages/record/dist/record.umd.cjs 379 kB +601 B (+0.16%)
packages/record/dist/record.umd.min.cjs 182 kB +261 B (+0.14%)
packages/replay/dist/replay.cjs 388 kB +577 B (+0.15%)
packages/replay/dist/replay.js 388 kB +577 B (+0.15%)
packages/replay/dist/replay.umd.cjs 391 kB +582 B (+0.15%)
packages/replay/dist/replay.umd.min.cjs 194 kB +252 B (+0.13%)
packages/rrweb-player/dist/rrweb-player.cjs 460 kB +577 B (+0.13%)
packages/rrweb-player/dist/rrweb-player.js 460 kB +577 B (+0.13%)
packages/rrweb-player/dist/rrweb-player.umd.cjs 463 kB +582 B (+0.13%)
packages/rrweb-player/dist/rrweb-player.umd.min.cjs 223 kB +252 B (+0.11%)
packages/rrweb-snapshot/dist/rrweb-snapshot.cjs 162 kB +577 B (+0.36%)
packages/rrweb-snapshot/dist/rrweb-snapshot.js 161 kB +577 B (+0.36%)
packages/rrweb-snapshot/dist/rrweb-snapshot.umd.cjs 165 kB +581 B (+0.35%)
packages/rrweb-snapshot/dist/rrweb-snapshot.umd.min.cjs 81.2 kB +252 B (+0.31%)
packages/rrweb/dist/rrweb.cjs 537 kB +588 B (+0.11%)
packages/rrweb/dist/rrweb.js 537 kB +588 B (+0.11%)
packages/rrweb/dist/rrweb.umd.cjs 538 kB +593 B (+0.11%)
packages/rrweb/dist/rrweb.umd.min.cjs 262 kB +252 B (+0.1%)
packages/utils/dist/utils.cjs 6.07 kB +600 B (+10.97%) ⚠️
packages/utils/dist/utils.js 5.57 kB +586 B (+11.75%) ⚠️
packages/utils/dist/utils.umd.cjs 7.1 kB +604 B (+9.3%) 🔍
packages/utils/dist/utils.umd.min.cjs 3.98 kB +266 B (+7.15%) 🔍
ℹ️ View Unchanged
Filename Size
packages/packer/dist/base-B40z8PPs.cjs 18.3 kB
packages/packer/dist/base-B40z8PPs.umd.cjs 19.4 kB
packages/packer/dist/base-B40z8PPs.umd.min.cjs 10.1 kB
packages/packer/dist/base-BrE4jft0.js 18.2 kB
packages/packer/dist/pack.cjs 347 B
packages/packer/dist/pack.js 285 B
packages/packer/dist/pack.umd.cjs 2.25 kB
packages/packer/dist/pack.umd.min.cjs 1.73 kB
packages/packer/dist/packer.cjs 257 B
packages/packer/dist/packer.js 136 B
packages/packer/dist/packer.umd.cjs 1.28 kB
packages/packer/dist/packer.umd.min.cjs 1.25 kB
packages/packer/dist/unpack.cjs 769 B
packages/packer/dist/unpack.js 702 B
packages/packer/dist/unpack.umd.cjs 1.79 kB
packages/packer/dist/unpack.umd.min.cjs 1.57 kB
packages/plugins/rrweb-plugin-canvas-webrtc-record/dist/rrweb-plugin-canvas-webrtc-record.cjs 37.6 kB
packages/plugins/rrweb-plugin-canvas-webrtc-record/dist/rrweb-plugin-canvas-webrtc-record.js 37.5 kB
packages/plugins/rrweb-plugin-canvas-webrtc-record/dist/rrweb-plugin-canvas-webrtc-record.umd.cjs 38.7 kB
packages/plugins/rrweb-plugin-canvas-webrtc-record/dist/rrweb-plugin-canvas-webrtc-record.umd.min.cjs 22.9 kB
packages/plugins/rrweb-plugin-canvas-webrtc-replay/dist/rrweb-plugin-canvas-webrtc-replay.cjs 34.3 kB
packages/plugins/rrweb-plugin-canvas-webrtc-replay/dist/rrweb-plugin-canvas-webrtc-replay.js 34.2 kB
packages/plugins/rrweb-plugin-canvas-webrtc-replay/dist/rrweb-plugin-canvas-webrtc-replay.umd.cjs 35.4 kB
packages/plugins/rrweb-plugin-canvas-webrtc-replay/dist/rrweb-plugin-canvas-webrtc-replay.umd.min.cjs 21.2 kB
packages/plugins/rrweb-plugin-sequential-id-record/dist/rrweb-plugin-sequential-id-record.cjs 681 B
packages/plugins/rrweb-plugin-sequential-id-record/dist/rrweb-plugin-sequential-id-record.js 548 B
packages/plugins/rrweb-plugin-sequential-id-record/dist/rrweb-plugin-sequential-id-record.umd.cjs 1.76 kB
packages/plugins/rrweb-plugin-sequential-id-record/dist/rrweb-plugin-sequential-id-record.umd.min.cjs 1.47 kB
packages/plugins/rrweb-plugin-sequential-id-replay/dist/rrweb-plugin-sequential-id-replay.cjs 933 B
packages/plugins/rrweb-plugin-sequential-id-replay/dist/rrweb-plugin-sequential-id-replay.js 820 B
packages/plugins/rrweb-plugin-sequential-id-replay/dist/rrweb-plugin-sequential-id-replay.umd.cjs 2.01 kB
packages/plugins/rrweb-plugin-sequential-id-replay/dist/rrweb-plugin-sequential-id-replay.umd.min.cjs 1.61 kB
packages/replay/dist/style.css 2.45 kB
packages/replay/dist/style.min.css 1.97 kB
packages/rrdom-nodejs/dist/rrdom-nodejs.cjs 132 kB
packages/rrdom-nodejs/dist/rrdom-nodejs.js 131 kB
packages/rrdom-nodejs/dist/rrdom-nodejs.umd.cjs 134 kB
packages/rrdom-nodejs/dist/rrdom-nodejs.umd.min.cjs 66.9 kB
packages/rrdom/dist/rrdom.cjs 150 kB
packages/rrdom/dist/rrdom.js 149 kB
packages/rrdom/dist/rrdom.umd.cjs 152 kB
packages/rrdom/dist/rrdom.umd.min.cjs 74.1 kB
packages/rrweb-player/dist/events.js 159 kB
packages/rrweb-player/dist/global.css 240 B
packages/rrweb-player/dist/style.css 5.57 kB
packages/rrweb-player/dist/style.min.css 5 kB
packages/rrweb/dist/style.css 2.45 kB
packages/rrweb/dist/style.min.css 1.97 kB
packages/types/dist/types.cjs 5.63 kB
packages/types/dist/types.js 5.38 kB
packages/types/dist/types.umd.cjs 6.66 kB
packages/types/dist/types.umd.min.cjs 3.41 kB

@Juice10 Juice10 merged commit 88ea2d0 into rrweb-io:master Feb 6, 2025
6 checks passed
gnpaone added a commit to Midpath-Software/rrweb that referenced this pull request Feb 7, 2025
* Fix up the 'should replace the existing DOM nodes on iframe navigation with `isAttachIframe`' test (rrweb-io#1636)

- it was working for me when the test was run in isolation (`-t` option), but when the entire cross-origin-iframes test was run, the change of iframe contents didn't seem to happen in time

* [chore]: Update actions/upload-artifact to v4 (rrweb-io#1643)

* update actions/upload-artifact to v4

---------

Co-authored-by: Eoghan Murray <[email protected]>

* Fix a code path where masking could be skipped on textareas (rrweb-io#1599)

* Fixes rrweb-io#1596

* [chore] Cache yarn packages for CI (rrweb-io#1646)

* [chore] Cache yarn packages for CI

* Cache yarn in release.yml

* [chore] Update deprecated download artifact on CI (rrweb-io#1647)

* I'm merging even though ESLint is stlll failing in Github Actions as I believe it's running actions _without_ this PR applied yet

* Fix env puppeteer error in cross-origin-iframes.test.ts (rrweb-io#1629)

* chore(ci): track bundle size (rrweb-io#1630)

* chore(ci): track bundle size

---------

Co-authored-by: pauldambra <[email protected]>

* Fix adapt css with split (rrweb-io#1600)

Fix for rrweb-io#1575 where postcss was raising an exception

* adapt the entire CSS as a whole in one pass with postcss, rather than adapting each split part separately
* break up the postcss output again and assign to individual text nodes (kind of inverse of splitCssText at record side)
* impose an upper bound of 30 iterations on the substring searches to preempt possible pathological behavior
* add tests to demonstrate the scenario and prevent regression

More technical details:
* Fix algorithm; checks against `ix_end` within loop were incorrect when `ix_start` was bigger than zero.  
* Fix that length check against wrong array was causing 'should record style mutations with multiple child nodes and replay them correctly' test to fail. 
Note on last point: I haven't looked into things more deeply than that the test was complaining about missing .length after `replayer.pause(1000);`

* Warn instead of fail on exceptions thrown from postcss (rrweb-io#1580)

* postcss was introduced in rrweb-io#1458 for use within adaptCssForReplay
* rrweb-io#1600 fixes the main case where invalid css could be introduced when if valid css from the output of `sheet.cssRules` was split according to how it was split across text nodes of the <style>
* the guard introduced here is still useful as we likely in future will switch to capturing the raw stylesheet contents (both <style> and <link>), at which point we will be much less confident of getting valid css

* Fix splitCssText again (rrweb-io#1640)

Fixes a browser 'lock up' at record time due to a presence of large amounts of css in <style> elements, which are split over multiple text nodes, which triggers the new code added in rrweb-io#1437 (see that PR for full explanation of why this all exists).  rrweb-io#1437 was not written with performance in mind as it was believed to be an edge case, but things like Grammarly browser extension (rrweb-io#1603) among other scenarios were triggering pathological behavior, some of which was solved in rrweb-io#1615.
See also rrweb-io#1640 (comment) for further discussion.

* Fix the case when there are multiple matches and we end up not finding a unique one - just go with the best guess when there are many splits by looking at the previous chunk's size
* Also add '0px' -> '0' stylesheet normalization, which also fixes the sample problem in a different way
* Add new test and modify it so that it can trigger a failure in the absence of the '0px' normalization; there may be other unknown ways of triggering a similar bug, so ensure that the primary 'best guess' method doesn't suffer a regression
* Leverage the 'best guess' method so that we can quit after 100 iterations trying to find a unique substring; hopefully this bit along with the `iterLimit` already added will prevent any future pathological cases.

Failing example extracted from large files identified by Paul D'Ambra (Posthog) ... see comment from MartinWorkfully: PostHog/posthog-js#1668

* fix: move patch function into utils to improve bundling (rrweb-io#1631)

* fix: move patch function into utils to improve bundling

---------

Co-authored-by: pauldambra <[email protected]>
Co-authored-by: Justin Halsall <[email protected]>

---------

Co-authored-by: Eoghan Murray <[email protected]>
Co-authored-by: Kevin Townsend <[email protected]>
Co-authored-by: Justin Halsall <[email protected]>
Co-authored-by: Paul D'Ambra <[email protected]>
Co-authored-by: pauldambra <[email protected]>
Co-authored-by: John Henry Gunther <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants